-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Добавлен метод загрузки визитов веб-аналитики #184
Добавлен метод загрузки визитов веб-аналитики #184
Conversation
3c252c2
to
ccf6580
Compare
src/ResourceGroup/WebAnalytics.php
Outdated
/** @var VisitsUploadResponse $response */ | ||
$response = $this->sendRequest( | ||
RequestMethod::POST, | ||
'web-analytics/client-ids/upload', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Должно быть '/web-analytics/visits/upload'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а почему тогда к примеру в clientIdsUpload
без этого слеша?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, слешь не нужно, копипаст сработал. А вот вместо client-ids
должно быть visits
.
EOF; | ||
|
||
$entity = new Visit(); | ||
$entity->createdAt = '2023-12-06T12:00:00'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
У нас формат Y-m-d H:i:s
заложен. Если использовать текущий, нужно править логику апишки.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
исправил, подогнал под нужный формат
{ | ||
"success": true | ||
} | ||
EOF; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут всё корректно?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вообще да, но поправил на более привычный вид
$json = '{"success": true}';
* @JMS\Type("string") | ||
* @JMS\SerializedName("pageDepth") | ||
*/ | ||
public $pageDepth; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
У текущей и у $pageViews
и $visitLength
должны быть числовые типы.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
У $pageDepth
не исправлено.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
поправил
src/ResourceGroup/WebAnalytics.php
Outdated
* $client = SimpleClientFactory::createClient('https://test.retailcrm.pro', 'apiKey'); | ||
* | ||
* $entity = new Visit(); | ||
* $entity->createdAt = '2023-12-06T12:00:00'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут также формат другой, у нас Y-m-d H:i:s
.
->withBody($json); | ||
|
||
$client = TestClientFactory::createClient($mock->getClient()); | ||
$response = $client->webAnalytics->visitsUpload($request); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ниже ворнинги нужно проверить.
ccf6580
to
118ef18
Compare
* @JMS\Type("int") | ||
* @JMS\SerializedName("pageViews") | ||
*/ | ||
public $pageViews; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
У нас мы меняли это название на countViews
. В аннотации тоже нужно поменять.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
поправил
/** | ||
* @var int | ||
* | ||
* @JMS\Type("int") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут тоже для единообразия integer
можно.
* @JMS\Type("string") | ||
* @JMS\SerializedName("pageDepth") | ||
*/ | ||
public $pageDepth; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
У $pageDepth
не исправлено.
118ef18
to
2d0ed4c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #184 +/- ##
============================================
+ Coverage 84.26% 84.30% +0.04%
- Complexity 1105 1108 +3
============================================
Files 168 169 +1
Lines 4194 4205 +11
============================================
+ Hits 3534 3545 +11
Misses 660 660 ☔ View full report in Codecov by Sentry. |
No description provided.